-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: add auth-test-server for OAuth conformance testing #1384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- MCP server with Bearer token authentication - Uses SDK's requireBearerAuth middleware - Validates tokens via AS introspection endpoint (RFC 7662) - Serves Protected Resource Metadata at /.well-known/oauth-protected-resource - Designed for server auth conformance tests
|
commit: |
| }); | ||
|
|
||
| // Handle GET requests - SSE streams for sessions (also requires auth) | ||
| app.get('/mcp', bearerAuth, async (req: Request, res: Response) => { |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
This route handler performs
authorization
This route handler performs
authorization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 23 hours ago
In general, to fix missing rate limiting on HTTP handlers that perform authorization or expensive operations, add a rate-limiting middleware (ideally using a well-known library) and apply it to the sensitive routes. The middleware should limit the number of requests per client (e.g., per IP) over a time window, returning an error (often 429) when the limit is exceeded. This reduces the risk of denial-of-service by preventing individual clients from overwhelming the server.
For this file, the least invasive and most standard fix is to use the express-rate-limit package. We will:
- Import
express-rate-limitnear the other imports. - Define a
mcpRateLimiterinstance with reasonable defaults (e.g., number of requests per 15 minutes) and a message appropriate for this auth test server. - Apply
mcpRateLimiterspecifically to the/mcproutes (POST, GET, DELETE) by inserting it in the middleware chain immediately afterbearerAuth(so unauthorized requests are still cheap while authenticated requests are rate-limited). This addresses all three alert variants because they are all on the same route family (/mcp) with the same missing rate limiting concern.
We only touch src/conformance/auth-test-server.ts. The changes are:
- Add
import rateLimit from 'express-rate-limit';along with existing imports. - Inside
startServer, after setting upconst app = express();(in the part of the file not fully shown but present), defineconst mcpRateLimiter = rateLimit({ ... }). - Update three route definitions:
app.post('/mcp', bearerAuth, adminScopeCheck, async ...)becomesapp.post('/mcp', bearerAuth, mcpRateLimiter, adminScopeCheck, async ...).app.get('/mcp', bearerAuth, async ...)becomesapp.get('/mcp', bearerAuth, mcpRateLimiter, async ...).app.delete('/mcp', bearerAuth, async ...)becomesapp.delete('/mcp', bearerAuth, mcpRateLimiter, async ...).
This preserves existing behavior (auth, scope checks, handler logic) while adding rate limiting to the authenticated routes only.
-
Copy modified line R27 -
Copy modified lines R309-R317 -
Copy modified line R383 -
Copy modified line R405
| @@ -24,6 +24,7 @@ | ||
| import express, { Request, Response, NextFunction } from 'express'; | ||
| import cors from 'cors'; | ||
| import { randomUUID } from 'crypto'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| // Extend Express Request type to include auth info from SDK middleware | ||
| declare module 'express' { | ||
| @@ -305,7 +306,15 @@ | ||
| ); | ||
|
|
||
| // Handle POST requests to /mcp with bearer auth and scope checking | ||
| app.post('/mcp', bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { | ||
| const mcpRateLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // Limit each client to 100 requests per windowMs | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| message: 'Too many requests to the MCP endpoint, please try again later.' | ||
| }); | ||
|
|
||
| app.post('/mcp', bearerAuth, mcpRateLimiter, adminScopeCheck, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| try { | ||
| @@ -371,7 +380,7 @@ | ||
| }); | ||
|
|
||
| // Handle GET requests - SSE streams for sessions (also requires auth) | ||
| app.get('/mcp', bearerAuth, async (req: Request, res: Response) => { | ||
| app.get('/mcp', bearerAuth, mcpRateLimiter, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| if (!sessionId || !transports[sessionId]) { | ||
| @@ -393,7 +402,7 @@ | ||
| }); | ||
|
|
||
| // Handle DELETE requests - session termination (also requires auth) | ||
| app.delete('/mcp', bearerAuth, async (req: Request, res: Response) => { | ||
| app.delete('/mcp', bearerAuth, mcpRateLimiter, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| if (!sessionId || !transports[sessionId]) { |
-
Copy modified lines R76-R78 -
Copy modified line R80
| @@ -73,5 +73,8 @@ | ||
| }, | ||
| "resolutions": { | ||
| "strip-ansi": "6.0.1" | ||
| } | ||
| }, | ||
| "dependencies": { | ||
| "express-rate-limit": "^8.2.1" | ||
| } | ||
| } |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
| }); | ||
|
|
||
| // Handle DELETE requests - session termination (also requires auth) | ||
| app.delete('/mcp', bearerAuth, async (req: Request, res: Response) => { |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
This route handler performs
authorization
This route handler performs
authorization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 23 hours ago
In general, the fix is to add a rate-limiting middleware (e.g., using express-rate-limit) and apply it to routes that perform authentication/authorization and other potentially expensive operations. This middleware should be configured with a reasonable time window and maximum number of requests per client (typically per IP) and then inserted into the middleware chain before the existing bearerAuth and handler functions.
For this file, the best low-impact fix is:
- Import
express-rate-limitat the top ofsrc/conformance/auth-test-server.ts. - Define a limiter instance (e.g.,
const mcpRateLimiter = rateLimit({...})) near where the Express app is configured. - Apply the limiter to the
/mcproutes that perform authorization:POST /mcp,GET /mcp, andDELETE /mcp. The existing functionality is preserved; we only add an extra middleware parameter beforebearerAuthso that if the request exceeds the limit, it is rejected early.
Concretely:
- Add
import rateLimit from 'express-rate-limit';alongside other imports. - After the Express
appis created (wherever that is defined in the same file) or near the route definitions, define a limiter, for example:
const mcpRateLimiter = rateLimit({
windowMs: 60 * 1000,
max: 60,
standardHeaders: true,
legacyHeaders: false
});- Update the three route handler registrations:
app.post('/mcp', mcpRateLimiter, bearerAuth, adminScopeCheck, async (req, res) => { ... });
app.get('/mcp', mcpRateLimiter, bearerAuth, async (req, res) => { ... });
app.delete('/mcp', mcpRateLimiter, bearerAuth, async (req, res) => { ... });This directly addresses all alert variants, as they all concern the /mcp handler using bearerAuth without rate limiting.
-
Copy modified line R27 -
Copy modified lines R309-R316 -
Copy modified line R382 -
Copy modified line R404
| @@ -24,6 +24,7 @@ | ||
| import express, { Request, Response, NextFunction } from 'express'; | ||
| import cors from 'cors'; | ||
| import { randomUUID } from 'crypto'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| // Extend Express Request type to include auth info from SDK middleware | ||
| declare module 'express' { | ||
| @@ -305,7 +306,14 @@ | ||
| ); | ||
|
|
||
| // Handle POST requests to /mcp with bearer auth and scope checking | ||
| app.post('/mcp', bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { | ||
| const mcpRateLimiter = rateLimit({ | ||
| windowMs: 60 * 1000, // 1 minute | ||
| max: 60, // limit each IP to 60 requests per window | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| }); | ||
|
|
||
| app.post('/mcp', mcpRateLimiter, bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| try { | ||
| @@ -371,7 +379,7 @@ | ||
| }); | ||
|
|
||
| // Handle GET requests - SSE streams for sessions (also requires auth) | ||
| app.get('/mcp', bearerAuth, async (req: Request, res: Response) => { | ||
| app.get('/mcp', mcpRateLimiter, bearerAuth, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| if (!sessionId || !transports[sessionId]) { | ||
| @@ -393,7 +401,7 @@ | ||
| }); | ||
|
|
||
| // Handle DELETE requests - session termination (also requires auth) | ||
| app.delete('/mcp', bearerAuth, async (req: Request, res: Response) => { | ||
| app.delete('/mcp', mcpRateLimiter, bearerAuth, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| if (!sessionId || !transports[sessionId]) { |
-
Copy modified lines R76-R78 -
Copy modified line R80
| @@ -73,5 +73,8 @@ | ||
| }, | ||
| "resolutions": { | ||
| "strip-ansi": "6.0.1" | ||
| } | ||
| }, | ||
| "dependencies": { | ||
| "express-rate-limit": "^8.2.1" | ||
| } | ||
| } |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
| // Configure CORS to expose Mcp-Session-Id header for browser-based clients | ||
| app.use( | ||
| cors({ | ||
| origin: '*', |
Check warning
Code scanning / CodeQL
Permissive CORS configuration Medium
permissive or user controlled value
- Add admin-action tool requiring 'admin' scope - Add scope-checking middleware for privileged tools - Returns 403 insufficient_scope for missing admin scope - Add scopes_supported to PRM response
| ); | ||
|
|
||
| // Handle POST requests to /mcp with bearer auth and scope checking | ||
| app.post('/mcp', bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
This route handler performs
authorization
This route handler performs
authorization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 23 hours ago
In general, to fix missing rate limiting on an Express route that performs authentication/authorization and potentially expensive work, you add a rate-limiting middleware. A common approach is to use a well-known library like express-rate-limit, configure a sensible window and max requests, and apply it either globally or specifically to the sensitive route(s). This ensures that even if an attacker can hit the endpoint, they can’t do so at an unbounded rate.
For this specific /mcp POST handler, the best minimal fix is:
- Import
express-rate-limit. - Define a rate limiter specifically for the
/mcpendpoint (so we don’t accidentally affect other routes like the metadata endpoint). - Insert the limiter middleware into the POST route’s middleware chain, alongside
bearerAuthandadminScopeCheck, without changing their behavior.
Concretely, in src/conformance/auth-test-server.ts:
-
Add a new import near the existing imports:
import rateLimit from 'express-rate-limit';. -
After
const app = express();and before the route definitions, define a limiter, for example:const mcpRateLimiter = rateLimit({ windowMs: 60 * 1000, // 1 minute max: 60, // limit each IP to 60 requests per window standardHeaders: true, legacyHeaders: false });
This is conservative but still reasonable for a test server; adjust values if the project has a standard.
-
Update the
/mcproute definition to includemcpRateLimiter:app.post('/mcp', mcpRateLimiter, bearerAuth, adminScopeCheck, async (req: Request, res: Response) => {
This leaves all auth and request handling logic intact while ensuring the route is rate-limited as required.
-
Copy modified line R27 -
Copy modified lines R281-R287 -
Copy modified lines R315-R316
| @@ -24,6 +24,7 @@ | ||
| import express, { Request, Response, NextFunction } from 'express'; | ||
| import cors from 'cors'; | ||
| import { randomUUID } from 'crypto'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| // Extend Express Request type to include auth info from SDK middleware | ||
| declare module 'express' { | ||
| @@ -277,6 +278,13 @@ | ||
| const app = express(); | ||
| app.use(express.json()); | ||
|
|
||
| const mcpRateLimiter = rateLimit({ | ||
| windowMs: 60 * 1000, // 1 minute | ||
| max: 60, // limit each IP to 60 requests per minute | ||
| standardHeaders: true, | ||
| legacyHeaders: false | ||
| }); | ||
|
|
||
| // Configure CORS to expose Mcp-Session-Id header for browser-based clients | ||
| app.use( | ||
| cors({ | ||
| @@ -304,8 +312,8 @@ | ||
| } | ||
| ); | ||
|
|
||
| // Handle POST requests to /mcp with bearer auth and scope checking | ||
| app.post('/mcp', bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { | ||
| // Handle POST requests to /mcp with bearer auth, scope checking, and rate limiting | ||
| app.post('/mcp', mcpRateLimiter, bearerAuth, adminScopeCheck, async (req: Request, res: Response) => { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| try { |
-
Copy modified lines R76-R78 -
Copy modified line R80
| @@ -73,5 +73,8 @@ | ||
| }, | ||
| "resolutions": { | ||
| "strip-ansi": "6.0.1" | ||
| } | ||
| }, | ||
| "dependencies": { | ||
| "express-rate-limit": "^8.2.1" | ||
| } | ||
| } |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
Summary
Adds an auth-test-server to the conformance test suite for testing server-side OAuth implementation.
What's New
src/conformance/auth-test-server.ts- MCP server with OAuth authenticationsrc/conformance/README.mdwith documentationFeatures
requireBearerAuthmiddleware for authentication/.well-known/oauth-protected-resourceMCP_CONFORMANCE_AUTH_SERVER_URLenvironment variableUsage
# Start with a fake auth server MCP_CONFORMANCE_AUTH_SERVER_URL=http://localhost:3000 \ npx tsx src/conformance/auth-test-server.tsRelated
This server is used by the conformance repo's server auth tests (modelcontextprotocol/conformance#105).